Resolve current stack of stuck lock holders - #1701
Conversation
The lock tracker records where a stuck lock was first acquired, but not what its holder is doing now — during an incident the waiters are easy to find in a goroutine dump while the blocked holder is anonymous. Record the holder's goroutine id on acquisition (~0.3ns via goid) and add PopulateHolderStacks to resolve holders' current stacks from a single runtime.Stack snapshot at scan time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 1414a20 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
paulwe
left a comment
paulwe
left a comment
There was a problem hiding this comment.
does this meaningfully impact overhead?
|
|
||
| func (t *lockTracker) trackUnlock() { | ||
| if t != nil { | ||
| gid := goid.Get() |
There was a problem hiding this comment.
is this guaranteed to be the same g that acquired the lock? this + the max 16 holders for rw locks seems a bit sketchy
There was a problem hiding this comment.
Given the comment above in trackLock, guessing it is one of the goroutines that is holding the lock. So, it should ideally be in one of these slots or an overflow. Wonder what is the case where the overflow removal also fails and a random one is removed from the slots.
There was a problem hiding this comment.
It is possible that a lock is acquired from 1 goroutine and then released from another - that feels like a hacky pattern that should probably be avoided. Maybe something that we specifically log on in staging
Track every holder gid (up to 8 concurrent RWMutex readers) in atomic slots instead of only the first, so PopulateHolderStacks resolves all holders' current stacks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
a8664f3 to
c53ceae
Compare
Every unlock now removes exactly one holder entry: its own gid if present, else an overflow credit, else an arbitrary slot (same policy go-deadlock uses for cross-goroutine unlocks). Replaces the racy wipe-all-slots-at-zero, making occupied slots + overflow == holders an exact invariant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
boks1971
left a comment
boks1971
left a comment
There was a problem hiding this comment.
this will be very useful. Thank you @erikhortsch
|
|
||
| func (t *lockTracker) trackUnlock() { | ||
| if t != nil { | ||
| gid := goid.Get() |
There was a problem hiding this comment.
Given the comment above in trackLock, guessing it is one of the goroutines that is holding the lock. So, it should ideally be in one of these slots or an overflow. Wonder what is the case where the overflow removal also fails and a random one is removed from the slots.
Mutex has exactly one holder and trackLock/trackUnlock run while it is held, so its bookkeeping needs no atomics — a plain gid store replaces the slot claim, and cross-goroutine unlock just clears it. The slot array, overflow accounting, and eviction heuristics now exist only for RWMutex readers in lock_tracker_rw.go, where holders are genuinely concurrent. Wrapped Mutex Lock/Unlock: 8.7ns -> 6.1ns (pre-tracking baseline 6.2ns, native 4.0ns). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Lock paths guard tracking behind one nil check with the post-acquire bookkeeping deferred; unlock paths use a bare atomic load (loadTracker) instead of the full lazy-init, since a lock being unlocked was locked first. StuckLock gains HolderStrength (exclusive vs shared readers). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lazyInitTracker now CASes the constructed tracker directly: construct is pure allocation, the CAS winner registers, losers are left to the GC. This removes the mid-init sentinel state, the runtime spin linknames, and the per-type registration/finalizer duplication. Both tracker types live in one registry — rwLockTracker extends lockTracker, so the scan reads the base through every ref and toStuckLock gathers reader slots when the rw flag is set. window is a weakRefList method with the cursor as a field. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3 participants
Motivation
During a recent incident, the lock tracker's stuck-lock logs told us where goroutines were waiting, but not where the holder was blocked. The tracker records the acquisition stack (
FirstLockedAtStack), which points at where the lock was grabbed — not at the call the holder is currently stuck in. And since holder goroutine ids aren't recorded, their stacks can't be picked out of a goroutine dump either: waiters are self-evident (parked insync.Mutex.Lock), holders are anonymous.Changes
Single-holder tracking for exclusive locks (
lock_tracker.go):Mutexhas exactly one holder andtrackLock/trackUnlockrun while it's held, so the holder gid is a plain field — no atomics except the waiter count, and a cross-goroutine unlock simply clears it. Lock paths guard tracking behind one nil check; unlock paths use a bare atomic load (loadTracker) instead of the lazy-init.Read-holder tracking as an RW extension (
lock_tracker_rw.go):RWMutexwrite holds use the embedded single-holder path; concurrent readers use atomic gid slots (up to 8, overflow-counted). Reader accounting removes exactly one entry per RUnlock — own gid, else overflow credit, else arbitrary eviction (the same cross-goroutine-unlock policy go-deadlock uses; compareHolderGoroutineIDswithNumGoroutineHeldto detect overflow).Holder stacks:
PopulateHolderStacks(stuck)resolves each holder's current stack from a singleruntime.Stack(buf, true)snapshot. The snapshot stops the world, so consumers run it once per detection episode rather than on every scan.StuckLock API:
HolderGoroutineIDs(),HolderStacks(), andHolderStrength()(exclusive vs shared readers).Cost (Apple M5 Pro, per Lock/Unlock pair)
Holder tracking on exclusive locks is effectively free; the RW read path pays goid+CAS each way.
Testing
Holder-stack resolution, cross-goroutine unlock, reader overflow, and holder-strength tests; full
./utilssuite passes with-race.🤖 Generated with Claude Code